Skip to content

refactor(quinn-proto): Use ArrayRangeSet in favor of RangeSet#2445

Open
matheus23 wants to merge 1 commit into
quinn-rs:mainfrom
n0-computer:matheus23/use-array-range-set
Open

refactor(quinn-proto): Use ArrayRangeSet in favor of RangeSet#2445
matheus23 wants to merge 1 commit into
quinn-rs:mainfrom
n0-computer:matheus23/use-array-range-set

Conversation

@matheus23

Copy link
Copy Markdown
Contributor

This seems to improve performance by ~8-10% in local benchmarks for me.

This seems to improve performance by ~8-10% in local benchmarks for us.

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and simple, and seems plausible?

@matheus23

Copy link
Copy Markdown
Contributor Author

I would hold off on merging this too eagerly. For me this improves some benchmarks by quite a bit, but others also regress.

Mostly this tells me that overall CPU use seems to depend on the range set performance characteristics.

@Ralith

Ralith commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

Another concern that bears some thought is the hazard of a CPU DoS attack. A peer can exert substantial control over the number of ranges in these fields, and the B tree structure is less sensitive to total size, though how much this matters depends on the exact accesses we're performing.

@Ralith

Ralith commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

#2422 will also improve some B-tree operations, which might tilt the balance.

@gretchenfrage

Copy link
Copy Markdown
Collaborator

Another concern that bears some thought is the hazard of a CPU DoS attack. A peer can exert substantial control over the number of ranges in these fields, and the B tree structure is less sensitive to total size, though how much this matters depends on the exact accesses we're performing.

Could consider an enum that tries to make it an array range set, but switches over to a btree range set if the number of ranges grows beyond some small constant?

@Ralith

Ralith commented Nov 30, 2025

Copy link
Copy Markdown
Collaborator

That should work, yeah, at the cost of a little more complexity and boilerplate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants